release: v0.8.0-alpha.1 — module migration, AX-6 testify replacement, AX-10 scaffold#4
release: v0.8.0-alpha.1 — module migration, AX-6 testify replacement, AX-10 scaffold#4
Conversation
… AX-10 scaffold Consolidates the v0.8.0-alpha.1 release work for core/go-session. * Module path migration: forge.lthn.ai/core/go-session -> dappco.re/go/session. All sibling dappco.re/go/* deps pinned to v0.8.0-alpha.1. * AX-6 testify removal: replaced testify assertions with stdlib testing patterns across the test suite. Removes external test framework dep in favour of the universal Core test contract. * AX-6 annotation: intrinsic banned imports annotated where their use is structurally required (boundary functions, justified by comment). * AX-10 scaffold: tests/cli/session/Taskfile.yaml + test driver for the session CLI surface. * Stale path cleanup: minor housekeeping of references to the pre-migration module path. Refs: RFC-CORE-008-AGENT-EXPERIENCE.md (AX-1, AX-6, AX-10) Co-authored-by: Athena <athena@lthn.ai> Co-authored-by: Hephaestus <hephaestus@lthn.ai> Co-authored-by: Cladius Maximus <cladius@lthn.ai>
📝 WalkthroughWalkthroughThe pull request removes the testify testing framework as an external dependency and replaces it with local test helper functions. It renames the Go module from Changes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test_helpers_test.go`:
- Around line 170-178: assertInDelta currently lets calls with NaN or negative
delta silently pass; update the function (assertInDelta) to first validate
inputs by using math.IsNaN on want and got and call t.Fatalf with a clear
message if either is NaN, then check that delta is not negative and t.Fatalf if
it is, and only after those guards compute the absolute difference and compare
to delta; also add the math import if missing and reuse testContext(msgAndArgs)
in the failure messages for consistent context.
In `@tests/cli/session/main.go`:
- Around line 72-82: The requireNoError function currently panics using string
concatenation (panic(msg + ": " + err.Error())) which bypasses the repo-standard
error construction; change it to build the error via core.E(...) and then panic
that error (e.g., panic(core.E(msg, err)) or similar wrapper signature used in
this codebase), keeping the human message and the original error attached;
update imports if necessary to reference core, and leave require(ok bool, msg
string) untouched unless it also should use core.E for consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26403adf-2e8f-4fe3-902d-1fa3b7e9d7a7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
analytics.goanalytics_test.goconventions_test.gocore_helpers.gogo.modhtml.gohtml_test.goparser.goparser_test.gosearch.gosearch_test.gotest_helpers_test.gotests/cli/session/Taskfile.yamltests/cli/session/main.govideo.govideo_test.go
| func assertInDelta(t *testing.T, want, got, delta float64, msgAndArgs ...any) { | ||
| t.Helper() | ||
| diff := want - got | ||
| if diff < 0 { | ||
| diff = -diff | ||
| } | ||
| if diff > delta { | ||
| t.Fatalf("%swant %v within %v, got %v", testContext(msgAndArgs), want, delta, got) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -i test_helpers_test.goRepository: dAppCore/go-session
Length of output: 84
🏁 Script executed:
# Get line count and examine the file around lines 170-178
wc -l test_helpers_test.go && echo "---" && sed -n '160,185p' test_helpers_test.goRepository: dAppCore/go-session
Length of output: 559
🏁 Script executed:
# Check current imports at top of file
head -30 test_helpers_test.goRepository: dAppCore/go-session
Length of output: 559
🏁 Script executed:
# Verify SPDX header presence
head -1 test_helpers_test.goRepository: dAppCore/go-session
Length of output: 100
assertInDelta silently passes when want or got is NaN.
In Go, all comparisons with NaN evaluate to false (including NaN > delta), so the function bypasses failure checks when either parameter is NaN. Guard NaN values explicitly. Also validate that delta is non-negative.
Proposed fix
import (
+ "math"
"reflect"
"testing"
func assertInDelta(t *testing.T, want, got, delta float64, msgAndArgs ...any) {
t.Helper()
- diff := want - got
- if diff < 0 {
- diff = -diff
- }
- if diff > delta {
+ if math.IsNaN(want) || math.IsNaN(got) || math.IsNaN(delta) || delta < 0 {
+ t.Fatalf("%sinvalid delta comparison (want=%v, got=%v, delta=%v)", testContext(msgAndArgs), want, got, delta)
+ }
+ if math.Abs(want-got) > delta {
t.Fatalf("%swant %v within %v, got %v", testContext(msgAndArgs), want, delta, got)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test_helpers_test.go` around lines 170 - 178, assertInDelta currently lets
calls with NaN or negative delta silently pass; update the function
(assertInDelta) to first validate inputs by using math.IsNaN on want and got and
call t.Fatalf with a clear message if either is NaN, then check that delta is
not negative and t.Fatalf if it is, and only after those guards compute the
absolute difference and compare to delta; also add the math import if missing
and reuse testContext(msgAndArgs) in the failure messages for consistent
context.
| func require(ok bool, msg string) { | ||
| if !ok { | ||
| panic(msg) | ||
| } | ||
| } | ||
|
|
||
| func requireNoError(err error, msg string) { | ||
| if err != nil { | ||
| panic(msg + ": " + err.Error()) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify established non-test error-construction conventions in this repository.
rg -n --type=go -g '!**/*_test.go' '\bcoreerr\.E\s*\('
rg -n --type=go -g '!**/*_test.go' '\bcore\.E\s*\('Repository: dAppCore/go-session
Length of output: 1620
🏁 Script executed:
#!/bin/bash
# Check imports in files that use core.E() to understand the alias
echo "=== Checking video.go imports ==="
head -30 video.go | cat -n
echo ""
echo "=== Checking parser.go imports ==="
head -30 parser.go | cat -nRepository: dAppCore/go-session
Length of output: 3144
Use the established error construction helper instead of panic with string concatenation.
The panic(msg + ": " + err.Error()) pattern bypasses the repository-standard error wrapper and loses structured operation context. Route errors through core.E() to match established patterns across the codebase.
♻️ Proposed refactor
import (
"os"
"strings"
"time"
+ core "dappco.re/go/core"
session "dappco.re/go/session"
)
@@
-func requireNoError(err error, msg string) {
+func requireNoError(op string, err error, msg string) {
if err != nil {
- panic(msg + ": " + err.Error())
+ panic(core.E(op, msg, err))
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func require(ok bool, msg string) { | |
| if !ok { | |
| panic(msg) | |
| } | |
| } | |
| func requireNoError(err error, msg string) { | |
| if err != nil { | |
| panic(msg + ": " + err.Error()) | |
| } | |
| } | |
| import ( | |
| "os" | |
| "strings" | |
| "time" | |
| core "dappco.re/go/core" | |
| session "dappco.re/go/session" | |
| ) | |
| func require(ok bool, msg string) { | |
| if !ok { | |
| panic(msg) | |
| } | |
| } | |
| func requireNoError(op string, err error, msg string) { | |
| if err != nil { | |
| panic(core.E(op, msg, err)) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cli/session/main.go` around lines 72 - 82, The requireNoError function
currently panics using string concatenation (panic(msg + ": " + err.Error()))
which bypasses the repo-standard error construction; change it to build the
error via core.E(...) and then panic that error (e.g., panic(core.E(msg, err))
or similar wrapper signature used in this codebase), keeping the human message
and the original error attached; update imports if necessary to reference core,
and leave require(ok bool, msg string) untouched unless it also should use
core.E for consistency.
1 participant
Summary
Consolidates the v0.8.0-alpha.1 release work for core/go-session.
Headline changes
forge.lthn.ai/core/go-session→dappco.re/go/session. All siblingdappco.re/go/*deps pinned tov0.8.0-alpha.1.tests/cli/session/Taskfile.yaml+ test driver for the session CLI surface.Refs
RFC-CORE-008-AGENT-EXPERIENCE.md(AX-1, AX-6, AX-10)Test plan
go test ./...passestests/cli/sessionTaskfile drivers exercise the CLI surfaceCo-authored-by: Athena athena@lthn.ai
Co-authored-by: Hephaestus hephaestus@lthn.ai
Co-authored-by: Cladius Maximus cladius@lthn.ai
Summary by CodeRabbit
Release Notes
Chores
Tests
Refactor